Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🛠️ use local lit #58

Closed
wants to merge 1 commit into from
Closed

🛠️ use local lit #58

wants to merge 1 commit into from

Conversation

iantrich
Copy link
Contributor

fixes #57

@TheZoker
Copy link
Contributor

Can confirm, that this fixes my issue!

@kalkih
Copy link
Owner

kalkih commented Jan 15, 2019

Cool!
This would certainly reduce the size of the card, however it would make it incompatible for people running versions of HA prior to 0.80.0 (where Lit was introduced).

@iantrich
Copy link
Contributor Author

Very true. Could include an additional ‘or’

@ZinkNotTheMetal
Copy link

I changed this in the top line of my file and didn't have any luck, still the same issue even after a restart of HomeAssistant

@iantrich
Copy link
Contributor Author

@ZinkNotTheMetal doesn't require a restart, you need to clear your browser cache, however

@ZinkNotTheMetal
Copy link

@iantrich thank you sir!

@kalkih
Copy link
Owner

kalkih commented Jan 15, 2019

Very true. Could include an additional ‘or’

Yes, but the build process would then import LitElement anyway.

If it wasn't for the lost backwards compatibility I would merge this now.
But then again, people running Lovelace UI and Custom cards are probably keeping their HA fairly up to date, or what do you think?

@TheZoker
Copy link
Contributor

Yea I guess these people have a update to date HA. 0.80 is also 5 version behind, so for me this step would be ok

@kalkih kalkih changed the base branch from master to dev January 15, 2019 15:54
@iantrich
Copy link
Contributor Author

@kalkih I think a note saying that future releases are only supported by 0.80+ is fair.

@kalkih
Copy link
Owner

kalkih commented Jan 16, 2019

Thanks for your feedback.
I'll merge this at a later stage, I want to release version 1.0.0 first, so that people unwilling to update their HA at least get that version.

@kalkih kalkih added the enhancement New feature or request label Jan 16, 2019
@iantrich
Copy link
Contributor Author

iantrich commented Jan 16, 2019

@kalkih
I did some tests on the dev branch of HA, we upgraded lit to 2.0.0-rc.2, and it does not appear to be playing nice :( Haven't done enough testing to know for sure that it can't be worked around, but not being able to ensure the version of lit your card is using means that it is more likely that core updating will break custom cards. Specifying the actual version needed, without ^, is probably the better update for now.
@thomasloven, do you have anything to add to this discussion? Whatever we decide here is going to be what I recommend to other lit custom cards as well. Thanks all.

@kalkih
Copy link
Owner

kalkih commented Jan 16, 2019

Interesting, that could indeed be an issue and cause to cards break, especially with lit and lit-element still being young and developed quite extensively.

For now, my build process solves the issues related to externally loaded dependencies by bundling them in, the drawback being the additional size of the card.

I'm going to remove the ability to use the source directly in the next release, since people still seem to do it, and it's causing unnecessary confusion and issues.

@iantrich
Copy link
Contributor Author

I like the bundling idea and might see about adding to mine and suggesting it to others

@iantrich iantrich closed this Jan 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants